-
Notifications
You must be signed in to change notification settings - Fork 37
feat: ws support proxy #145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
感谢同学PR;如果直接从httpInstance上取agent,看起来会把wsclient和httpInstance耦合起来(agent还会分http和https的情况),如果给wsClient增加一个agent的构造参数好点儿? |
wsclient 内部 也会通过 httpInstance 发送请求, 也是需要走代理的。 所以是 共用的同一个实例。 agent 分 http 和 https, 但现实中 HTTP 已经标记为不安全的, 且现在向 飞书 请求的时候,也不会走 HTTP, 所以 直接取的 httpsAgent 来做为 agent 。
本来想这么写的,后来想想 不会有 HTTP 的场景(本地开发 用HTTP时, 可以 不配置代理 就好了),就改成现在 PR 中的样子了。 给 WebSocket 配置代理的代码参考的这里: |
默认取httpInstance的agent也是合理的,现在确实用了httpInstance来发起连接; 如果有需要将httpInstance和wsClient拆分开的场景,我们也可以增加构造参数来支持,且不引入break change; 这样改OK哈,不过还有2个地方:
|
好的,我再更新一下。 |
faf34c3
to
b827ff8
Compare
单独定义了一个 HttpInstance 的原因是有的同学会自定义httpInstance,比如使用node-fetch,这样的话类型和参数就不能完全和axios对齐;我们只需要定义需要的接口即可。 同理,这里我们需要自定义一下defaults,避免和axios耦合:
|
好的,那我再修改一下。 |
0b34f31
to
26a9aab
Compare
… service through a proxy
根据刚才的回复,如果加 defaults 其实和 axios 绑定了。 所以 根据 前面的讨论, 感觉加个 agent 的可选参数, 更好一点。 麻烦你再看看, 现在这样改可以吗? 更新一下示例代码:
|
包发好了: |
Pass the agent parameter to WebSocket to support accessing the target service through a proxy.
full example code: